feat(deno): redis diagnostics channel based integration for deno#21087
Conversation
size-limit report 📦
|
) Refactor the redis-dc integration logic into core/src/integrations, and create a Deno integration that uses the same patterns. Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration just adds `_sentrySpan` onto the data in a RedisTracingChannelFactory which is passed to the core utility. Add deno-redis e2e test.
f176fd8 to
0fc320c
Compare
1048f32 to
72be1fa
Compare
) Refactor the redis-dc integration logic into core/src/integrations, and create a Deno integration that uses the same patterns. Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration just adds `_sentrySpan` onto the data in a RedisTracingChannelFactory which is passed to the core utility. Add deno-redis e2e test.
11525d8 to
a8d3cb7
Compare
8226aaf to
5863e6c
Compare
) Refactor the redis-dc integration logic into core/src/integrations, and create a Deno integration that uses the same patterns. Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration just adds `_sentrySpan` onto the data in a RedisTracingChannelFactory which is passed to the core utility. Add deno-redis e2e test.
5863e6c to
cd298fa
Compare
|
👋 @mydea, @andreiborza — Please review this PR when you get a chance! |
) Refactor the redis-dc integration logic into core/src/integrations, and create a Deno integration that uses the same patterns. Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration just adds `_sentrySpan` onto the data in a RedisTracingChannelFactory which is passed to the core utility. Add deno-redis e2e test.
) Refactor the redis-dc integration logic into core/src/integrations, and create a Deno integration that uses the same patterns. Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration just adds `_sentrySpan` onto the data in a RedisTracingChannelFactory which is passed to the core utility. Add deno-redis e2e test.
e46685b to
1f6c601
Compare
JPeer264
left a comment
There was a problem hiding this comment.
LGTM, just have one m which I want to get sorted before approving
|
👋 @mydea, @andreiborza — Please review this PR when you get a chance! |
I'm personally fine with this. Approved the other PR, waiting for @isaacs decision. |
) Refactor the redis-dc integration logic into core/src/integrations, and create a Deno integration that uses the same patterns. Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration just adds `_sentrySpan` onto the data in a RedisTracingChannelFactory which is passed to the core utility. Add deno-redis e2e test.
1f6c601 to
6a2f9b6
Compare
) Refactor the redis and ioredis diagnostics_channel integration logic into core/src/integrations, and create a Deno integration that uses the same patterns. Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration adds `_sentrySpan` onto the data in a RedisTracingChannelFactory which is passed to the core utility. Add deno-redis e2e test covering both redis clients. fix: #21221 fix: JS-2630
6a2f9b6 to
ebbbc65
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ebbbc65. Configure here.
JPeer264
left a comment
There was a problem hiding this comment.
LGTM. Lint is failing, but it looks like a small change.
| ): void { | ||
| const channel = tracingChannel<T>(channelName, data => { | ||
| const args = getCommandArgs(data); | ||
| const statement = safeSerialize(data.command, args); |
There was a problem hiding this comment.
m: tracing channels on redis already sanitize the command statements or is that a different thing?
https://github.com/redis/ioredis/blob/main/lib/tracing.ts#L10-L22
https://github.com/logaretm/node-redis/blob/master/packages/client/lib/client/tracing.ts#L39-L44
It was based off the one implemented by OTEL originally, and Redis team wanted Redis to own that part.
There was a problem hiding this comment.
Ooh, good catch. Yes, this was ported in from the otel implementation, and looks like we missed it the first time around, but it's just spinning extra CPU cycles unnecessarily, since the dc publishers already do this for us.
There was a problem hiding this comment.
This also means we don't have to handle Uint8Array/Buffer args, which simplifies it even further, actually.

Refactor the redis and ioredis diagnostics_channel integration logic into core/src/integrations, and create a Deno integration that uses the same patterns.
Instead of the @sentry/opentelemetry/tracing-channel, the Deno integration adds
_sentrySpanonto the data in a RedisTracingChannelFactory which is passed to the core utility.Add deno-redis e2e test covering both redis clients.
fix: #21221
fix: JS-2630